Replace format() function with PY3 fstrings #1087
Conversation
asv/plugins/summarylist.py
Outdated
| @@ -52,8 +52,7 @@ def publish(cls, conf, repo, benchmarks, graphs, revisions): | |||
| pretty_name = benchmark['pretty_name'] | |||
|
|
|||
| if idx is not None: | |||
| pretty_name = '{0}({1})'.format(pretty_name, | |||
| ", ".join(benchmark_param)) | |||
| pretty_name = f'{pretty_name}({", ".join(benchmark_param)})' | |||
There was a problem hiding this comment.
This is fine, but I don't think we're improving readability that much. I forgot to mention in the issue I think, but for non-trivial code, would be better to first assign to a variable, and then have the f-string, but with the variable name, not with the whole expression. In this case it's not that complex a simple join, but having quotes and commas on the expression, makes the f-string quite unreadable in my opinion.
There was a problem hiding this comment.
Thanks i had thought about this,makes total sense
asv/plugins/virtualenv.py
Outdated
| util.check_call([ | ||
| sys.executable, | ||
| "-mvirtualenv", | ||
| "-p", | ||
| self._executable, | ||
| self._path], env=env) | ||
|
|
||
| log.info("Installing requirements for {0}".format(self.name)) | ||
| log.info("Installing requirements for {self.name}") |
There was a problem hiding this comment.
Not an f-string. We need to be careful with that, since it's very likely that not flake8 and not the tests will warn us when we miss the f to make an f-string, and we'll lose the value of the variables in the string.
|
Thanks @dorothykiz1, really nice |
xref #1077